Fixed merged PR filter shows less than 10 entries #181
Conversation
✅ Deploy Preview for github-spy ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
WalkthroughThe changes update the GitHub data fetching logic by switching to the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant TrackerPage
participant useGitHubAuth
participant useGitHubData
participant OctokitAPI
User->>TrackerPage: Changes tab/filter/page or submits form
TrackerPage->>useGitHubAuth: Get Octokit instance
TrackerPage->>useGitHubData: fetchData(username, page, perPage, type, state)
useGitHubData->>OctokitAPI: Search issues/PRs with filters
OctokitAPI-->>useGitHubData: Return filtered results
useGitHubData-->>TrackerPage: Update issues/PRs state
TrackerPage-->>User: Render updated table
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes found. Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Note 🔌 MCP (Model Context Protocol) integration is now available in Early Access!Pro users can now connect to remote MCP servers under the Integrations page to get reviews and chat conversations that understand additional development context. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
🎉 Thank you @Abhay056 for your contribution. Please make sure your PR follows https://github.com/GitMetricsLab/github_tracker/blob/main/CONTRIBUTING.md#-pull-request-guidelines
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/hooks/useGitHubAuth.ts (1)
11-14: Trim the token before instantiating OctokitIf the user pastes a PAT with a trailing space / newline, authentication silently fails. A one-liner avoids that:
- return new Octokit({ auth: token }); + return new Octokit({ auth: token.trim() });src/hooks/useGitHubData.ts (1)
33-43: Useis:instead ofstate:for open/closed to match GitHub search qualifiersWhile
state:opensometimes works, the documented qualifier isis:open/is:closed. Safer string building:- query += state === 'merged' ? ` is:merged` : ` state:${state}`; + query += state === 'merged' + ? ' is:merged' + : ` is:${state}`;This avoids query-syntax regressions if GitHub tightens parsing.
src/pages/Tracker/Tracker.tsx (1)
289-310: Minor CPU win: memoisefilteredDatafor large result sets
filterDataruns on every render; with title/date filters it can re-scan up to 100 items/time. Wrap inuseMemokeyed on[currentRawData, searchTitle, selectedRepo, startDate, endDate]to avoid redundant work.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
package.json(1 hunks)src/hooks/useGitHubAuth.ts(2 hunks)src/hooks/useGitHubData.ts(2 hunks)src/pages/Tracker/Tracker.tsx(10 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/pages/Tracker/Tracker.tsx (2)
src/hooks/useGitHubData.ts (1)
GitHubItem(7-7)src/pages/ContributorProfile/ContributorProfile.tsx (1)
ContributorProfile(11-95)
🔇 Additional comments (1)
src/pages/Tracker/Tracker.tsx (1)
73-90: Missing dependencies will violate React-hook exhaustive-deps rule
fetchDataandusernameare referenced inside thisuseEffectbut omitted from the dependency array. ESLint’sreact-hooks/exhaustive-depswill flag this and, more importantly, username changes won’t trigger a refetch unless the user resubmits the form.- }, [page, tab, issueFilter, prFilter]); + }, [page, tab, issueFilter, prFilter, username, fetchData]);If you intentionally excluded them, add an inline
// eslint-disable-next-line react-hooks/exhaustive-depswith a rationale.
| "@emotion/styled": "^11.11.0", | ||
| "@mui/icons-material": "^5.15.6", | ||
| "@mui/material": "^5.15.6", | ||
| "@octokit/rest": "^22.0.0", |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove redundant Octokit packages to avoid version drift
With @octokit/rest added, the existing "octokit" meta-package (Line 25) is now redundant and may pull in a different, transitive @octokit/rest version. Keeping both can bloat install size and introduce subtle mismatches.
Either drop "octokit" or justify why both are required; keeping only one source of the REST client is the safer default.
🤖 Prompt for AI Agents
In package.json at line 19, the dependency "@octokit/rest" is added while the
"octokit" meta-package is also present at line 25, causing redundancy and
potential version conflicts. Remove the "octokit" package from the dependencies
to avoid pulling in multiple versions of the REST client and reduce install
size, unless there is a specific reason to keep both.
1 participant
Related Issue
Details of Fix
Type of Change
Summary by CodeRabbit